-
Notifications
You must be signed in to change notification settings - Fork 331
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[StoreKit 2] Raise error if no AppTransaction and no Transaction present #4054
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this PR covers the case where AppTransaction is missing and there are no transactions present for the syncPurchases
flow, but I think we need to do something to address the case of this happening in the purchase flow as well to be safe.
Perhaps adding something like this to CustomerAPI.post
, around line 91?
if receipt == .empty && appTransaction == nil {
// Call completion with error
}
@@ -1129,6 +1129,14 @@ private extension PurchasesOrchestrator { | |||
return | |||
} | |||
|
|||
// We dont have an AppTransactionJWS, and no transaction, return error | |||
if appTransactionJWS == nil { | |||
completion?(.failure(ErrorUtils.storeProblemError( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we make sure that this is dispatched on the main thread?
initiationSource: .restore) | ||
fail("Expected error") | ||
} catch { | ||
expect(error).to(matchError(ErrorCode.storeProblemError)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to make sure that this error is thrown from the main thread?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not here afaik, we'll figure out a different way to test this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was just doing some similar stuff on the retry PR, one thing that you might be able to do to help is to mock out the OperationDispatcher
and then verify that invokedDispatchOnMainThread
is true. While you're not directly testing the thread, it's close in that it's verifying how the OperationDispatcher is executing the work.
When we don't have an AppTransaction, and the list of transactions is empty as well, this is likely an store issue, so we return an error and let the developer retry the operation.
Checklist
purchases-android
and hybridsMotivation
Description